Skip to content

Conversation

@jakelewis3d
Copy link

This PR replaces my previous PR #19 which can be ignored and closed.

Audio Settings Dialog II

1)audio interface and audio input selection. Light subsampling removed.
Under the command menu, the audio setting dialog allows user choice of audio interface and audio input. Default values work best. I'm not sure these should be exposed on release builds as some combinations do not produce a signal. For Windows though, using https://www.vb-audio.com/Cable/ is the easiest way to playback recordings, so it's needed for dev builds.
Sample rate can be selected too, but is not guaranteed to be available, again default values work best.

The subsampling of Light algorithm has been removed, as that can now be selected independently above. Light algorithm is now just the lowered buffer sizes without noise reduction.

2)removed round() methods from inner loops to outer loops in compute_phase and compute_waveform.
These were using 5% according to vtune. No change in output.

3)SIMD alignment of all fftw buffers.
There may be a reason for it, but some fftwf buffers were using malloc rather than the DWORD alingned memory that allows SIMD operations. http://www.fftw.org/doc/SIMD-alignment-and-fftw_005fmalloc.html#SIMD-alignment-and-fftw_005fmalloc Can't say I noticed any huge gains though.

4)cyan / magenta colored events a waveforms for tic / toc
Rather than all the events colored white, the tics and tocs are now colored cyan and magenta, as are the waveforms and period background. This makes it much easier to reduce beat errors as the colors switch places on the papertrace if you go too far. Off-centric escape wheels cycles are more evident too.
To maintain backward compatibility, this was done via tocs having negative eventtimes.
Methods
int absEventTime(int eventTime); int event_is_TIC_or_TOC(int eventTime);
are used to convert timings back to regular time, and to tell if a time is a tic or toc.

5)Changed Lift Angle spinner to allow floating points. 0.5 increments.
Trivial change, but occasionally some lift angles are not integers.

6)Low and High Pass ranges added to Audio Settings Dialog
Added range controls to audio settings dialog so user can play with filter cutoffs. These values are saved between sessions. The order and effect of the filters has not been altered.

Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
…ase and compute_waveform

These round() methods were profiling at 5%!

Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
@jakelewis3d
Copy link
Author

in algo.c the numeric shift needs to be reduced to 15, or there is an integer rollover in calibration mode. Sorry about that.
#define PERIOD_SHIFT 15

Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
@jakelewis3d
Copy link
Author

Papertrace magnification
Added Mag+ and Mag- buttons to the controls beneath papertrace. These allow zooming in and out of events.

I just so tired of having to edit the names each time.

Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
@jakelewis3d
Copy link
Author

Snapshot presets
I've grown tired of repeatedly labeling snapshots for testing in positions. Now it's a click away.

Signed-off-by: jakelewis3d <jakelewis3d@gmail.com>
@jakelewis3d
Copy link
Author

My bad, I'd left a few #defs in that I need to build in eclipse IDE. Now removed.

Makefile.am Outdated
src/audio_settings.c \
src/audio.h \
src/gtk/gtkHelper.c \
src/gtk/gtkHelper.h \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not valid syntax and is rejected by automake. Line 20 should not have whitespace and there should be no backslash continuation on the final line.

*/

#include <gtk/gtk.h>
#include "gtk/gtkHelper.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this is the correct path for your header. This is relative to the location of the current file. So this means "src/gtk/gtk/gtkHelper.h", which is not the correct location. It should be "gtkHelper.h".

But I don't know if you really need a new directory here. The interface code, still in src, is just as much Gtk code as this is.

* Created on: Oct 24, 2019
* Author: jlewis
* This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License version 2 as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment formatting is mangled here.

cairo_pattern_get_rgba (cairocolor,
&red,&green,&blue,&alpha);
char rgbaStr[1024];
sprintf(rgbaStr, "rgba(%f,%f,%f,%f)", 255*red, 255*green, 255*blue, alpha);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should at least use snprintf(). But glib has functions for safe strings now that would be better.

// spin button

GtkWidget * createSpinButtonContainer(const gchar*name,
double min, double max,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem like your indent here is correct. Maybe you are viewing this file with non-standard top stops?

src/interface.c Outdated
}


void console(char *format,...)//this is better for eclipse development
Copy link
Contributor

@xyzzy42 xyzzy42 Jan 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem related to audio settings dialog. Perhaps a different commit? I don't actually see it used.

src/interface.c Outdated
w->snapshot_POS_button[4] = gtk_button_new_with_label("9U");
w->snapshot_POS_button[5] = gtk_button_new_with_label("12U");
for (int i = 0; i < POSITIONS; i++) {
GtkWidget *button = w->snapshot_POS_button[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

static const char* const labels[] = {"DD", "DU", "3U", ... };
GtkWidget *button = w->snapshot_POS_button[i] = gtk_button_new_with_label(labels[i]);

if( strcmp(gtk_button_get_label(b) , MAG_INC)==0)
paperstrip_zoom_var *= MAG_SCALE;
else
paperstrip_zoom_var /= MAG_SCALE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace errors

}

gboolean interface_setFilter(struct main_window *w, gboolean bLpf, double freq){
if(w->computer){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would w->computer not be set?

return 1;

PaStreamParameters inputParameters;
inputParameters.channelCount = 2; // Stereo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of setting these here? And why stereo? The audio is converted to mono so wouldn't mono be better to capture?

inputParameters.channelCount = info.channels = channels;

if(inputParameters.device == Pa_GetDefaultInputDevice()){
debug("Pa_OpenDefaultStream dev:%d channels:%d\n",inputParameters.device, inputParameters.channelCount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason to not just use Pa_OpenStream() once, for any device? Is there some difference in calling Pa_OpenDefaultStream().


#include "audio.h"

#include <gtk/gtkHelper.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path for this header is wrong. This isn't a system header.

@xyzzy42
Copy link
Contributor

xyzzy42 commented Jan 2, 2021

Opening the audio setting dialog always causes tg to crash.

CONFIG_FIELDS(LOAD);

w->audioInputStr = g_key_file_get_string(w->config_file, "tg", "audioInputStr", NULL);
w->audioInterfaceStr= g_key_file_get_string(w->config_file, "tg", "audioInterfaceStr", NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the config file doesn't have this key, then w->audioInterfaceStr is NULL and the audio dialog will crash.

Comment on lines +80 to +94
static const char * filterDev( PaDeviceIndex devIndex, int nominal_sample_rate){
const PaDeviceInfo *info = Pa_GetDeviceInfo(devIndex);
if (info->maxInputChannels > 0){
PaStreamParameters inputParameters;
inputParameters.channelCount = 2; // Stereo
inputParameters.sampleFormat = paFloat32;
// inputParameters.suggestedLatency = ;
inputParameters.hostApiSpecificStreamInfo = NULL;
inputParameters.device = devIndex;
if( nominal_sample_rate < 0 || Pa_IsFormatSupported( &inputParameters, NULL, nominal_sample_rate ))
return info->name;
debug("%d is not supported by dev %s \n", nominal_sample_rate, info->name);
}
return NULL;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be working well. Few things:

It returns NULL if the sample rate is supported, since Pa_IsFormatSupported() will returns a PaError and paNoError is 0. So if nominal_sample_rate is not less than zero, it appears to get the support backward.
Unless it is supposed to return NULL if the format is supported? It's good to write documentation when submitting code to a project, so that other's can understand it.

It requires a stereo microphone. My mics are mono, so they get rejected.

It doesn't work to query a device for format support while using the device at the same time with PortAudio's ALSA driver.

#ifndef SRC_AUDIO_H_
#define SRC_AUDIO_H_

#include "tg.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But tg.h includes audio.h. Doesn't make sense have audio include tg and tg include audio.

Comment on lines +646 to +648
double phase = fmod(event, sweep) / sweep; // 0.0 -> 1.0
double cycle = strip_width * ( 0.5 + (phase - 0.5) * zoom_factor);
int column = floor(fmod(cycle, strip_width));
Copy link
Contributor

@xyzzy42 xyzzy42 Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This zoom only works if sweep is a integer multiple of strip_width. If not, what happens is phase = fmod(event, sweep) will increase/decrease until it reaches zero and wraps around. At the same time, column = fmod(cycle, strip_width) is also wrapping around as the path cross the chart edges. If column doesn't wrap around at the same time as phase, then the path will jump from some point in the middle of the chart, by an amount equal to fmod(sweep, strip_width). The don't wrap at the same time if sweep isn't an exact multiple of strip_width.

In my PR that adds a smooth beat length zoom I wrote a totally new algorithm that solves this problem.

@xyzzy42 xyzzy42 mentioned this pull request Mar 28, 2021
Signed-off-by: Jake Lewis <jakelewis3d@gmail.com>
Signed-off-by: Jake Lewis <jakelewis3d@gmail.com>
@jakelewis3d
Copy link
Author

I've addressed most of the issues itemized above that relate to the features:
removed round() methods from inner loops to outer loops in compute_phase and compute_waveform.
SIMD alignment of all fftw buffers.
cyan / magenta colored events a waveforms for tic / toc
Changed Lift Angle spinner to allow floating points. 0.5 increments
Predefined Snapshot button names

Features
audio interface and audio input selection. Light subsampling removed.
Low and High Pass ranges added to Audio Settings Dialog
Zoom on papertrace

have been implemented by others elsewhere, and can be ignored from this PR

calculation of beat error moved from first pulse (unlock), to third pulse (lock)
removed conversion of beat error to an absolute number.  Beat Error now shows + or -
@jakelewis3d jakelewis3d reopened this Jun 25, 2022
@jakelewis3d
Copy link
Author

Update (poorly labelled) algo.c address a problem I've seen many times whereby the accuracy of the amplitude detection has a direct effect on the reported beat error. For example in this instance the amplitude of the cyan tick has been miscalculated as around 280 degrees, whereas, from inspecting the waveform visually it should be around 250, (similar to the magenta waveform).
beat calculated from unlock.
This produces a beat error of 1.1ms ( which coincides with differences between the lower scale timings of the cyan and magenta first pulses).
The problem subsides when the first pulse is too faint to detect, as then the beat error is detected directly from the third pulse, but then jumps to a different value when the amplitude is briefly detected.
Here I have simply omitted any use of first pulse measurements being used for beat errors - it always uses the third pulse.

beat calculated from lock

The timing of the third (lock) pulse is generally more accurate and more reliable than the first (unlock) pulse, and I think this will give more robust beat error readings to less experienced uses. So which is theoretically correct - first or third pulse? Witschy claim the first, while I am claiming the third will be more robust, but it's actually the second pulse (the sound of the escape wheel imparting the power through the face of the palette jewel and onto the balance's impulse jewel AS IT PASSES THE MIDPOINT OF THE CYCLE), that would best dictate correct beat timings. Unfortunately, that second sound is generally not precise, but I'd suggest that both first and third pulses have equal divergence from this optimal timing, so we'd be better off using the one that is stronger and better defined.

The audio I used for these images (recorded with pre-amplified contact mic) is available at [https://horologyobsession.com/downloads/timegrapher/audiosamples/44100_21600_53degrees.wav]

@jakelewis3d
Copy link
Author

jakelewis3d commented Jun 25, 2022

update signed beat error.
Beat error is traditionally measured in (positive) millisecs, ignoring the contrasting cases when the tic->toc is longer than toc->tic or the reverse. This makes eliminating the beat error unnecessarily tricky as it approaches zero as an overshoot will register the same as an undershoot. By displaying the sign of the beat error the reading will remain coherent as long as the software does not lose track of the ticking. With the caveat that the software does not actually know which is tick and which is tock, and thus might initialize reversed, the same technique used to reduce the rate error can also be used for beat error - ie the "+" or "-" (or "f" and "s" on swiss watches) indicating the direction to manipulate the stud carrier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants